-
Notifications
You must be signed in to change notification settings - Fork 191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
allow specifying only one container #2121
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have a test for this?
Not sure if we need a test for that, the issue was mainly that I didn't have into account that someone can specify a container in a different syntax than the nf-core modules template 😓 |
Codecov Report
@@ Coverage Diff @@
## dev #2121 +/- ##
==========================================
- Coverage 71.43% 71.39% -0.04%
==========================================
Files 77 77
Lines 8350 8356 +6
==========================================
+ Hits 5965 5966 +1
- Misses 2385 2390 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
LGTM but have any of us actually tested it on a previously non-compliant but acceptable module? |
I tested one of the modules that only has one container, |
Only thing I can think of is what happens if the image names are really short and the docker/singularity are on the same line? Actually I think in that case we end up assuming there is only one container per-line and that the line starts with the url 😕 |
you can check the deepvariant modules or the gatk4/cnnscorevariants that use only one container and no conda is available |
Works for this type of modules 👍
In that case, it will check the number of double quotes, but only try to connect to the first container it finds. If a module is created with nf-core, even if the containers are short, they will be in two separate lines. For local modules, I will add a warning checking that. |
Close #2116
PR checklist
CHANGELOG.md
is updateddocs
is updated